Fixed texture clamping caused by texture coordinates being outside of (0,1) interval #188

Pasalc · 2023-11-04T16:00:19Z

Fix for issue #173.

Short description

Video gets clamped for 2659503088 wallpaper. Problem is caused by texture coordinates being out of boundaries, resulting in clamp.

Full description

Texture UV coordinates starts from 0 and ends at 1. We will change uend and see how it will affect video 2667198601.
If we have:

float ustart = 0.0f;
float uend = 1.0f;
float vstart = 1.0f;
float vend = 0.0f;

we will get full size video:

And if we have:

float ustart = 0.0f;
float uend = 0.5f;
float vstart = 1.0f;
float vend = 0.0f;

we will get only left halve of video:

And if we try to render texture out of this boundries:

float ustart = 0.0f;
float uend = 1.5f;
float vstart = 1.0f;
float vend = 0.0f;

texture will be clamped to the edge (because GL_CLAMP_TO_EDGE parametr in CFBO.cpp is set):

What caused problem

Previously, ustart/uend or vstart/vend could be less than 0 or bigger than 1 (for example ustart could be -0.16 and uend could be 1.15), resulting in texture clamping on left/right or top/bottom of window or screen(though I only had top/bottom clamp). By making them equal to 0 or 1 accordingly, we make sure that OpenGL renders whole texture with glDrawArrays and don't get out of boundaries so we have no clamping.

Before:

After:

Additional concerns

It probably shouldn't affect people from issue #81, but may be it's a good idea to ask if everything works fine for them.

Teddy-Kun · 2023-11-04T16:18:50Z

I can confirm this fixes the pixel artifacts, however I don't think this approach is always the best as this stretches the wallpaper to fit the size given by either the monitor or window. I believe that there could/should be a command line flag to change the behavior. On OLED screens I imagine it could be desirable to just leave the pixels outside completely black, whereas for my current personal wallpaper I would prefer cropping the sides to not stretch out the image. But to not undermine your work, this is 100% a lot better than before and the code definitely looks much cleaner and simpler to me!

Pasalc · 2023-11-05T17:58:10Z

Thank you for response! I would probably do as you suggested and add flags:

fit the size
leave the pixels outside completely black
if flag was not provided: preserve things as they are now

If i understand correctly, by

cropping the sides to not stretch out the image

you meant flag 2, but correct me if I got it wrong.

Pasalc · 2023-11-06T14:43:10Z

Added --clamp-strategy option, with stretch, border, repeat, clamp arguments. Can be shortend to -t and arguments to s, b, r, c. @Teddy-Kun for your case you should be able to write:
./linux-wallpaperengine <background_path/background_id> -t b
and you will get black borders instead of clamp.

Not that important, but I also had to add in WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp
#include <GL/glew.h>
or else I would get compile error:

In file included from /home/kermit/Projects/fun/linux-wallpaperengine/src/WallpaperEngine/Render/Drivers/CVideoDriver.h:3,
                 from /home/kermit/Projects/fun/linux-wallpaperengine/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.h:3,
                 from /home/kermit/Projects/fun/linux-wallpaperengine/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp:2:
/usr/include/GL/glew.h:85:2: error: #error gl.h included before glew.h
   85 | #error gl.h included before glew.h
      |  ^~~~~
/usr/include/GL/glew.h:97:2: error: #error glext.h included before glew.h
   97 | #error glext.h included before glew.h
      |  ^~~~~

Also repeat looks like this:

Pasalc · 2023-11-08T17:25:36Z

Just realized that this issue might be related to #90 , so I would add another flag option and make zoom fill another option too. For now I just made it default. Looks fine to me, but I'm not sure it really works correctly, so more thought must be put into that.
It looks like this for different window sizes(I don't know their exact size):

So yeah, it's either zoomed to full width or to full height.

Pasalc · 2023-11-13T17:44:43Z

Overview

So I added 2 flags:

--scaling <mode>	 Scaling mode for wallpaper. Can be stretch, fit, fill, default. Must be used before wallpaper provided.
                    		 Example: ./wallengine --scaling stretch --screen-root eDP-1 --bg 2667198601 --scaling fill --screen-root eDP-2 2667198602

--clamping <mode>	 Clamping mode for all wallpapers. Can be clamp, border, repeat. Enables GL_CLAMP_TO_EDGE, GL_CLAMP_TO_BORDER, GL_REPEAT accordingly. Default is clamp.

Scaling mode should work with different screens, so you could for example set stretch for one wallpaper and fill for other. You must provide --scaling option before wallpaper was provided.

Clamping mode apply to all screens, so you can't set it for each screen individually.

Code

Added new class CWallpaperState files:

Added CWallpaperState as new member in CWallpaper class.

CWallpaperState saves last viewport, projection, vflip and UVs values. Now if nothing have changed(viewport, projection, vflip) then old UVs coordinates are used (or more simple: UVs are cached).
If something have changed, then the value of m_textureUVsMode defines which scaling algorithm will be applied to recalculate UVs.

If any new scaling algorithm will be needed in the future, then it can be easily(I hope) added:

Add new value to enum class TextureUVsScaling in CWallpaperState.h
Provide new template specialization for your algorithm with template<> void CWallpaperState::updateTextureUVs<CWallpaperState::TextureUVsScaling::YOUR_ALGORITHM> in CWallpaperState.cpp
Add new switch statement in CWallpaperState::updateState(...)
Add new switch statement for case 't' in CApplicationContext.cpp

How Fit/Fill works

I think it's better to explain it with example with different sizes of wallpaper. Here we will only look on examples were only width is different, because the same logic applies to height.

Both equal
Imagine you have screen with sizes 2000x2000 and wallpaper with sizes 2000x2000. Then, with default scale(the one that is used on main now) you will have left=0;right=2000. Just as it should be.

Width is smaller
If screen sizes was 2000x2000 and wallpaper was 1000x2000, you would get left=-500; right=1500. So now you will get clamps, meaning wallpaper will zoom fit the screen.

Width is bigger
If screen sizes was 2000x2000 and wallpaper was 3000x2000, you would get left=500; right=2500. So now you will see only part of wallpaper, meaning wallpaper will zoom fill the screen.

The same principles applies if width is equal for screen and wallpaper, but the height differs.

Fit
In code screen is viewport and wallpaper is projection. So, if we get either viewport.width==projection.width && viewport.height>projection.height or viewport.height==projection.height && viewport.width>projection.width we will get zoom fit when using code for default scale algorithm for findingUVs coordinates. To make one of this conditions true, we can use this code:

const float m1 = static_cast<float>(viewportWidth) / projectionWidth;
const float m2 = static_cast<float>(viewportHeight) / projectionHeight;
const float m = std::min(m1,m2);
projectionWidth*=m;
projectionHeight*=m;

Fit
In code screen is viewport and wallpaper is projection. So, if condition is either viewport.width==projection.width && viewport.height<projection.height or viewport.height==projection.height && viewport.width<projection.width we will get zoom fit when using code for default scale algorithm for finding UVs coordinates. To make one of this conditions true, we can use same code as above, but max instead of min:

const float m1 = static_cast<float>(viewportWidth) / projectionWidth;
const float m2 = static_cast<float>(viewportHeight) / projectionHeight;
const float m = std::max(m1,m2);
projectionWidth*=m;
projectionHeight*=m;

Miscellaneous

Both --scaling and --clamping options are under case 't' in CApplicationContext.cpp, because there are a lot of letters already used for other options and I don't want to use even more if I can conserve.

Also added constexpr size_t customHash(const char* str) in CApplicationContext.cpp. Made it to have compile time hash for choosing options for scaling and clamping in switch statement. Could be done with simple if else statements, so maybe it's a bit of an overkill.

Pasalc · 2023-11-13T18:52:29Z

src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp

@@ -1,3 +1,4 @@
+#include <GL/glew.h>


Won't compile without that

Pasalc

Made simple review, hopefully it will be useful

Pasalc · 2023-11-13T18:53:37Z

src/WallpaperEngine/Application/CApplicationContext.cpp

@@ -28,9 +28,29 @@ struct option long_options[] = {
    { "noautomute", no_argument,            nullptr, 'm' },
    { "no-fullscreen-pause", no_argument,   nullptr, 'n' },
    { "disable-mouse", no_argument,         nullptr, 'e' },
+    { "scaling", required_argument,         nullptr, 't' },
+    { "clamping", required_argument,        nullptr, 't' },


Both use 't' to use less letters

Pasalc · 2023-11-13T18:54:35Z

src/WallpaperEngine/Application/CApplicationContext.cpp

+                size_t hash = customHash(optarg);
+                // Use a switch statement with the hash
+                switch (hash) {
+                    // --scale options


Here are scale options

Pasalc · 2023-11-13T18:54:44Z

src/WallpaperEngine/Application/CApplicationContext.cpp

+                    case customHash("default"):
+                        this->settings.render.window.scalingMode = WallpaperEngine::Render::CWallpaperState::TextureUVsScaling::DefaultUVs;
+                        break;
+                    // --clamp options


Here are clamp options

Pasalc · 2023-11-13T18:56:33Z

src/WallpaperEngine/Render/CWallpaperState.h

+    {
+    public:
+        // Scaling modes. Defines how UVs coordinates are calculated.
+        enum class TextureUVsScaling : uint8_t


Here are scaling options

Pasalc · 2023-11-13T18:57:20Z

src/WallpaperEngine/Render/CWallpaperState.cpp

+    }
+}
+
+


Below are specialization templates for all scaling options

Pasalc · 2023-11-13T19:07:02Z

src/WallpaperEngine/Render/CWallpaperState.cpp

+    }
+}
+
+template<> void CWallpaperState::updateTextureUVs<CWallpaperState::TextureUVsScaling::DefaultUVs>(){


This is copy of code that was used to calculate UVs before(in current main branch). Behavior(resulting UVs values) should be the same as old version

Pasalc · 2023-11-13T19:09:40Z

src/WallpaperEngine/Application/CApplicationContext.cpp

@@ -290,4 +349,8 @@ void CApplicationContext::printHelp (const char* route)
    sLog.out ("\t--set-property <name=value>\tOverrides the default value of the given property");
    sLog.out ("\t--no-fullscreen-pause\tPrevents the background pausing when an app is fullscreen");
    sLog.out ("\t--disable-mouse\tDisables mouse interactions");
+    sLog.out ("\t--scaling <mode>\t Scaling mode for wallpaper. Can be stretch, fit, fill, default. Must be used before wallpaper provided.\n\


Help entries for --scaling and --clamping

Pasalc · 2023-11-13T19:10:37Z

src/WallpaperEngine/Assets/ITexture.h

@@ -69,6 +69,7 @@ namespace WallpaperEngine::Assets
            NoInterpolation = 1,
            ClampUVs = 2,
            IsGif = 4,
+            ClampUVsBorder = 8,


Option for --clamping

Pasalc · 2023-11-13T19:11:28Z

src/WallpaperEngine/Application/CWallpaperApplication.cpp

@@ -322,13 +322,13 @@ namespace WallpaperEngine::Application
        for (const auto& it : this->m_backgrounds)
            context->setWallpaper (
                it.first,
-                WallpaperEngine::Render::CWallpaper::fromWallpaper (it.second->getWallpaper (), *context, *audioContext)
+                WallpaperEngine::Render::CWallpaper::fromWallpaper (it.second->getWallpaper (), *context, *audioContext, this->m_context.settings.general.screenScalings[it.first])


Use scalling chosen for this wallpaper

Pasalc · 2023-11-13T19:12:41Z

src/WallpaperEngine/Application/CWallpaperApplication.cpp

            );

        // set the default rendering wallpaper if available
        if (this->m_defaultBackground != nullptr)
            context->setDefaultWallpaper (WallpaperEngine::Render::CWallpaper::fromWallpaper (
-                this->m_defaultBackground->getWallpaper (), *context, *audioContext
+                this->m_defaultBackground->getWallpaper (), *context, *audioContext, this->m_context.settings.render.window.scalingMode


Use last chosen --scalling option

Pasalc · 2023-11-30T18:06:25Z

src/WallpaperEngine/Render/CWallpaper.cpp

-{
-    glBindBuffer (GL_ARRAY_BUFFER, this->m_texCoordBuffer);
-    glBufferData (GL_ARRAY_BUFFER, size, texCoords, GL_STATIC_DRAW);
-}


It wasn't used anywhere, so I removed it. Probably should have let it stay.

Almamu · 2023-12-12T00:06:24Z

The PR looks pretty good. There's slight adjustements I'd make to how scaling is calculated (mainly using templates to provide the different implementations), but I don't know when I'll have time to properly look at these, so accepting the PR as is. Thanks for your contribution and sorry for the delay in reviewing this!

Fixed not full texture rendering on viewport resulting in clamping

3dee616

Added --clamp-strategy option

f6bff67

This comment was marked as duplicate.

Sign in to view

Added zoom fill

4212095

Pasalc force-pushed the viewport-clamp branch 2 times, most recently from 4cebccd to f433218 Compare November 13, 2023 17:08

Pasalc force-pushed the viewport-clamp branch 2 times, most recently from be65768 to 9aeb402 Compare November 13, 2023 18:34

Added scaling and clamp support

11666ab

Pasalc force-pushed the viewport-clamp branch from 9aeb402 to 11666ab Compare November 13, 2023 18:39

Pasalc commented Nov 13, 2023

View reviewed changes

Pasalc commented Nov 30, 2023

View reviewed changes

Merge branch 'main' into viewport-clamp

13ba153

Almamu merged commit 51db296 into Almamu:main Dec 12, 2023
4 checks passed

Almamu mentioned this pull request Dec 12, 2023

Pixels stretched if WP resolution is different to screen resolution #173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed texture clamping caused by texture coordinates being outside of (0,1) interval #188

Fixed texture clamping caused by texture coordinates being outside of (0,1) interval #188

Pasalc commented Nov 4, 2023

Teddy-Kun commented Nov 4, 2023

Pasalc commented Nov 5, 2023 •

edited

Loading

Pasalc commented Nov 6, 2023

This comment was marked as duplicate.

Pasalc commented Nov 8, 2023

Pasalc commented Nov 13, 2023 •

edited

Loading

Pasalc Nov 13, 2023

Pasalc left a comment •

edited

Loading

Pasalc Nov 13, 2023 •

edited

Loading

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 13, 2023

Pasalc Nov 30, 2023

Almamu commented Dec 12, 2023

Fixed texture clamping caused by texture coordinates being outside of (0,1) interval #188

Fixed texture clamping caused by texture coordinates being outside of (0,1) interval #188

Conversation

Pasalc commented Nov 4, 2023

Fix for issue #173.

Short description

Full description

What caused problem

Additional concerns

Teddy-Kun commented Nov 4, 2023

Pasalc commented Nov 5, 2023 • edited Loading

Pasalc commented Nov 6, 2023

This comment was marked as duplicate.

Pasalc commented Nov 8, 2023

Pasalc commented Nov 13, 2023 • edited Loading

Overview

Code

How Fit/Fill works

Miscellaneous

Choose a reason for hiding this comment

Pasalc left a comment • edited Loading

Choose a reason for hiding this comment

Pasalc Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Almamu commented Dec 12, 2023

Pasalc commented Nov 5, 2023 •

edited

Loading

Pasalc commented Nov 13, 2023 •

edited

Loading

Pasalc left a comment •

edited

Loading

Pasalc Nov 13, 2023 •

edited

Loading